Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Temp fix for data loss caused by concurrent flush #96

Merged
merged 3 commits into from
Oct 16, 2019

Conversation

yiwu-arbug
Copy link
Collaborator

Summary:
Temporary fix for #93. Fixing it by waiting until no flush is ongoing before starting a GC job. The drawback is GC can starve if flush runs so frequent that there's no gap between flush jobs.

Test Plan:
See the new test. The test will fail with "Corruption: Missing blob file" error before the fix.

Signed-off-by: Yi Wu <yiwu@pingcap.com>

wait flush before gc

Signed-off-by: Yi Wu <yiwu@pingcap.com>

Fix hot fix

Signed-off-by: Yi Wu <yiwu@pingcap.com>

change to flush_hotfix branch

Signed-off-by: Yi Wu <yiwu@pingcap.com>

update sync point name

Signed-off-by: Yi Wu <yiwu@pingcap.com>

also apply fix to TEST_StartGC

Signed-off-by: Yi Wu <yiwu@pingcap.com>

change to flush_hotfix_6.4 branch

Signed-off-by: Yi Wu <yiwu@pingcap.com>

update test

Signed-off-by: Yi Wu <yiwu@pingcap.com>
@yiwu-arbug
Copy link
Collaborator Author

Need to merge tikv/rocksdb#126 before merging this one.

@yiwu-arbug yiwu-arbug removed the DNM label Oct 16, 2019
@Connor1996
Copy link
Member

Please merge latest master

Copy link
Member

@Connor1996 Connor1996 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@yiwu-arbug yiwu-arbug merged commit 0a3f87a into tikv:master Oct 16, 2019
@yiwu-arbug yiwu-arbug deleted the flush_tmpfix branch October 16, 2019 05:58
yiwu-arbug pushed a commit to yiwu-arbug/titan that referenced this pull request Oct 16, 2019
Summary:
Temporary fix for tikv#93. Fixing it by waiting until no flush is ongoing before starting a GC job. The drawback is GC can starve if flush runs so frequent that there's no gap between flush jobs.

Test Plan:
See the new test. The test will fail with "Corruption: Missing blob file" error before the fix.
yiwu-arbug pushed a commit that referenced this pull request Oct 16, 2019
Cherry-picking the following patches for fixing #93 
```
0a3f87a 2019-10-15 yiwu@pingcap.com     Temp fix for data loss caused by concurrent flush (#96)
8ac5003 2019-10-16 zbk602423539@gmail.. merge BackgroundGC with TEST_StartGC (#94)
b9915d9 2019-10-07 zbk602423539@gmail.. check nullptr (#91)
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants